fix(webview-ui): keep @ context picker open on folder selection (drilldown) #8289
Conversation
…ia Enter/Tab Insert folder mentions without trailing space so the picker remains active. Keep the picker open and immediately query folder children to enable keyboard drilldown. Add a focused test for folder drilldown behavior. Fixes #8076.
dosubot
bot
added
size:L
There was a problem hiding this comment.
Pull Request Overview
Improves the UX of the @ context picker by enabling keyboard drilldown navigation into folders. Previously, selecting a folder would close the picker, preventing users from exploring nested directory structures.
- Added special handling for folder selections to keep the picker open and append trailing slashes
- Implemented manual insertion logic to avoid trailing spaces that would close the context menu
- Added automated folder content search when drilling into directories
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| webview-ui/src/components/chat/tests/ChatTextArea.folder-drilldown.spec.tsx | New test file verifying folder drilldown behavior and picker state management |
| webview-ui/src/components/chat/ChatTextArea.tsx | Added special case handling for folder selections with manual insertion logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| setSearchQuery(nextQuery) | ||
|
|
||
| // Kick off a search to populate folder children | ||
| const reqId = Math.random().toString(36).substring(2, 9) |
There was a problem hiding this comment.
Using Math.random() for request ID generation could lead to collisions. Consider using crypto.randomUUID() or a more robust ID generation method to ensure uniqueness.
| const reqId = Math.random().toString(36).substring(2, 9) | |
| const reqId = crypto.randomUUID() |
| setTimeout(() => { | ||
| textAreaRef.current?.focus() | ||
| }, 0) |
There was a problem hiding this comment.
Using setTimeout with 0 delay is a code smell that suggests timing issues. Consider using requestAnimationFrame or a more deterministic approach to handle focus management.
| setTimeout(() => { | |
| textAreaRef.current?.focus() | |
| }, 0) | |
| requestAnimationFrame(() => { | |
| textAreaRef.current?.focus() | |
| }) |
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
| // Keep focus in the textarea for continued typing | ||
| setTimeout(() => { | ||
| textAreaRef.current?.focus() | ||
| }, 0) |
There was a problem hiding this comment.
Prefer requestAnimationFrame over a zero-delay timeout for deterministic focus after DOM updates.
| // - Insert without trailing space | ||
| // - Trigger a follow-up search to show folder contents | ||
| if (type === ContextMenuOptionType.Folder && value && textAreaRef.current) { | ||
| // Normalize folder path to end with a trailing slash |
There was a problem hiding this comment.
When constructing a folder path manually, escape spaces before insertion; unescaped whitespace after '@' can hide the picker.
| folderPath = folderPath + "/" | ||
| } | ||
|
|
||
| // Manually build insertion without a trailing space (more deterministic than insertMention) |
There was a problem hiding this comment.
Manual replacement (slicing/regex) is brittle for common path tokens; delegate to insertMention() and strip its trailing space to keep the menu open.
|
|
||
| // Keep the context menu open for drill-down | ||
| setShowContextMenu(true) | ||
| setSelectedType(null) |
There was a problem hiding this comment.
After keeping the picker open, reset the highlighted option (e.g., index 0) so Enter continues drilldown predictably.
| expect(lastMsg.query).toBe("/src/") | ||
| expect(typeof lastMsg.requestId).toBe("string") | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Add a test for folders with spaces: escaped insertion in the input and unescaped query in searchFiles.
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Changes Requested]
in Roo Code Roadmap
hannesrudolph
added
PR - Changes Requested
and removed
PR - Needs Preliminary Review
labels
…ape spaces for folder mentions and add coverage. Addresses review feedback by @daniel-lxs
|
Review completed. No new blocking issues identified. The implementation correctly addresses the reported issue of keeping the context picker open during folder drilldown. All critical concerns raised by previous reviewers have been addressed in the second commit:
The code correctly implements the folder drilldown feature with proper space handling, race condition prevention, and comprehensive test coverage. Mention @roomote in a comment to trigger your PR Fixer agent and make changes to this pull request. |
|
closing, will open new one. |
github-project-automation
bot
moved this from PR [Changes Requested]
to Done
in Roo Code Roadmap
Context picker UX improvement for @ path mentions.
Problem
Root cause
Solution
Tests
Local verification
Closes #8076
Important
Improves
ChatTextAreacontext picker UX by keeping it open on folder selection and adds tests for this behavior.handleMentionSelect()inChatTextArea.tsxnow keeps the context picker open for folder selections, normalizes folder paths to end with a slash, and inserts without a trailing space.searchFilesrequest for folder children immediately after selection.ChatTextArea.folder-drilldown.spec.tsxto test that the input becomes"@/src/"without a trailing space and that the picker remains open.searchFilesis called with the correct query for folders with and without spaces.escapeSpacestopath-mentionsimports inChatTextArea.tsx.This description was created by
for eaf3974. You can customize this summary. It will automatically update as commits are pushed.